Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix csv stats precision #1503

Merged
merged 3 commits into from
Aug 7, 2020
Merged

Conversation

vstepanov-lohika-tix
Copy link
Contributor

@vstepanov-lohika-tix vstepanov-lohika-tix commented Aug 5, 2020

Fix for #1501 issue
image

@codecov
Copy link

codecov bot commented Aug 5, 2020

Codecov Report

Merging #1503 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1503   +/-   ##
=======================================
  Coverage   81.19%   81.19%           
=======================================
  Files          27       27           
  Lines        2388     2388           
  Branches      367      367           
=======================================
  Hits         1939     1939           
  Misses        355      355           
  Partials       94       94           
Impacted Files Coverage Δ
locust/stats.py 89.01% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f8e64e...e198e82. Read the comment docs.

@vstepanov-lohika-tix vstepanov-lohika-tix changed the title Fix stats precision Fix csv stats precision Aug 5, 2020
@vstepanov-lohika-tix vstepanov-lohika-tix marked this pull request as ready for review August 6, 2020 07:04
s.avg_content_length,
s.total_rps,
s.total_fail_per_sec,
round(s.median_response_time) if s.median_response_time else s.median_response_time,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can define a function above such as _adjust_precision(stat) instead of fixing the precision at this scope. Moreover, having a separate function allows customization using different precision, if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are not intended to bring some new feature, but just to return an old view of csv report after regression.
Previously, data format was done also at this scope and was broken with an injection of csv_writer in 1.x.x version:
https://github.com/locustio/locust/pull/1428/files#diff-5d5f310549d6d596beaa43a1282ec49eL822
If we'll create a new function, it will be a complicated because of different data types in stats entries (time metrics, content metric, rate metrics). So it should be a some reason to add a new tangled function that probably will be used only once for this specific case.

Copy link
Collaborator

@cyberw cyberw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, thanks

@cyberw cyberw merged commit bac40b4 into locustio:master Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants